Thanks a lot, must have been a lot of work! Some suggestions.

All the credit for the approach and hard work goes to @costrouc

I'll unify the approach on skipping tests / fetching patches to upstream from PRs created to their repo. Thanks a lot for the quick feedback @flokli!

@flokli I think I covered all the comments/suggestions with the new commit. If I missed anything please let me know. Thanks in advance!

Not something that should stall this PR, but @ingenieroariel do you have a NixOS module in the pipeline aswell, or some quick notes on how to get it running?

It might be beneficial to have some NixOS tests that spins up airflow, and does some curl request to see it was able to do the initialization.

Thanks @flokli I have not yet thought about the module but it makes a lot of sense to get started on that after this gets merged.

Copying people from previous PRs who expressed interest in this being merged:
cc @offlinehacker @costrouc @cawilliamson @volth @risicle @costrouc @Ericson2314

@jonringer He created the package as part of #65026 and that addition is on a commit made by him that this PR respects.

I could add myself too if that is best practice (I did as much in the main airflow package) but it felt rude to just retroactively claim ownership.

Still want me to add myself? No problem doing so as I want to keep airflow working on unstable.

I guess I must have overlooked that this was based upon a previous PR, keeping him there is fine, but feel free to add yourself

I resolved the issues dealing with maintainership

Generally when adding python-modules, it's roughly one commit = one package addition. @FRidh how strictly do you want to adhere to that role, I think there's 18 or 19 packages being added

Generally when adding python-modules, it's roughly one commit = one package addition. @FRidh how strictly do you want to adhere to that role, I think there's 18 or 19 packages being added

I squashed the commits to make porting to latest master easier, each package folder is fine but the top-level/python-packages.nix file is problematic when a new package in master falls in the middle of two packages from this branch. If it is strictly needed I can re-create from scratch on top of master but commits may loose correct ownership.

I am not sure with a way this package is packaged, since you override all version requirements, how can you be sure things will work. I did a pull request a while back that used pypi2nix, it generated a large dependency file, but at least I was sure dependencies are right.

I am not sure with a way this package is packaged, since you override all version requirements, how can you be sure things will work. I did a pull request a while back that used pypi2nix, it generated a large dependency file, but at least I was sure dependencies are right.

This is a good point, I commented below the actual version we ended up choosing (based on what is available in NixOS):

--replace "flask>=1.1.0, <2.0" "flask" \  # got 1.1.1
--replace "flask-caching>=1.3.3, <1.4.0" "flask-caching" \ # got 1.7.2
--replace "flask-appbuilder>=1.12.5, <2.0.0" "flask-appbuilder" \ # got 2.1.6
--replace "pendulum==1.4.4" "pendulum" \ # got 2.0.5
--replace "cached_property~=1.5" "cached_property" \ # got 1.5.1
--replace "dill>=0.2.2, <0.3" "dill" \ # go 0.3.1.1
--replace "configparser>=3.5.0, <3.6.0" "configparser" \ # got 4.0.2
--replace "jinja2>=2.7.3, <=2.10.0" "jinja2" \ # got 2.10.3
--replace "funcsigs==1.0.0" "funcsigs" \ # got 1.0.2
--replace "flask-swagger==0.2.13" "flask-swagger" \ # got 0.2.14
--replace "python-daemon>=2.1.1, <2.2" "python-daemon" \ # got 2.2.3
--replace "alembic>=0.9, <1.0" "alembic" \ # got 1.2.1
--replace "markdown>=2.5.2, <3.0" "markdown" \ # 3.1.1
--replace "future>=0.16.0, <0.17" "future" \ # got 0.18.1
--replace "tenacity==4.12.0" "tenacity" \ # # got 5.1.1
--replace "text-unidecode==1.2" "text-unidecode" \ # got 1.3
--replace "tzlocal>=1.4,<2.0.0" "tzlocal" \ # got 2.0.0
--replace "sqlalchemy~=1.3" "sqlalchemy" \ # got 1.3.10
--replace "werkzeug>=0.14.1, <0.15.0" "werkzeug" # got 0.16.0

I do not expect markdown, sqalchemy, werkzeug or jinja2 to make wild changes on minor updates, but for those apps that made incompatible changes (like pendulum), patches to airflow have been provided. setup.py may look strict but from experience it is always a judgement call on the last person who created a release: in our case, the nixos package maintainer. After that, we only have the test suite to protect us and hopefully some module tests in the near future for integration testing.

If the versions of the dependencies do need diverge from whats on nixpkgs, then you can move this out of python-modules and create an application. At least there you can wrap the program with it's own pythonpath.

Having more than 1 version in python-modules isn't acceptable due to how environments are created with python packages (both will be present, only 1 will get used)

If the versions of the dependencies do need diverge from whats on nixpkgs, then you can move this out of python-modules and create an application. At least there you can wrap the program with it's own pythonpath.

Having more than 1 version in python-modules isn't acceptable due to how environments are created with python packages (both will be present, only 1 will get used)

This PR does not add more than one version of any packages, it attempts to reuse what is there. Both a top level application and a python-module are included here. For my project I do need a python-module on NixOS.

Thanks all for the comments and reviews. Should I do a one package per commit and force push the branch or is this okay as is?

not really sure, I was hoping @FRidh would make a more opinionated stance

@jonringer I went ahead and rebased one commit per package onto latest master, preserving original autorship.

do you mind editing your commit to be:

apache-airflow: 1.10.3 -> 1.10.5, enable python

also, your last commit, touches a lot of files

apache-airflow: 1.10.3 -> 1.10.5, enable python

Done.

The last commit, 7af3a95 touched around 10 different files, it should have been broken into ~9 commits on what packages it's bump, removing, or altering

@jonringer Urgh, you're right. Should we revert and do a new PR?

not entirely sure, but if there's a need to cherrypick or remove a package change, then we will have to revert that commit

reverting, then applying the 9 commits would probably be the most proper course of action

Sorry, lost track about this. I think reverting it causes even more noise. We probably won't backport things from here to 19.09, so it'll hopefully be fine…